-
Notifications
You must be signed in to change notification settings - Fork 452
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update trigger for DTD deployment job. #3585
Conversation
I don't have much experience with GitHub actions. Comparing your suggested changes with https://github.com/matsim-org/matsim-libs/blob/master/.github/workflows/deploy-on-pr-merge.yaml I see that the Otherwise... as I said, I don't have much experience, so it's okay for me. I guess, we'l have to try it out and see what happens. |
As far as I understand it, I changed it in a way, that the workflow is triggered on every closed PR, that modify something in the path 'matsim/src/main/resources/dtd/**'. This is currently true only few PRs. And then the job is only executed if the close was triggered by a merging. Do we have any PRs not merging into master? Unfortunately, I am also not the expert of it, but from some reading today, I am not totally sure, with that branches setting here. But yes, we can try to put it in again. |
The pipeline is now running, unfortunately something seems wrong with the paths for rsync: rsync: [sender] change_dir "/github/workspace/matsim/src/main/resources/dtd" failed: No such file or directory (2) Here is the full log: |
I think the path should probably be "/github/workspace/matsim-libs/matsim/src/main/resources/dtd |
Can you just change it, please? |
Now it works :) The final solution to ass the checkout command as well: #3597 |
The previous script was never run (which seems to be the cause for #3583), so there seems to be an issue with the triggers:
Therefore I changed the following:
@mrieser : Can you have a look in it, and if ok, merge it?
And then I assume, we need a PR, changing something in the dtd path, to see, if it works (and updating the ev's dtd at the same time) ...
Update: I have prepared such a PR: #3586